Skip to content

pkcs1v15: retrofit deterministic implicit rejection for decryption#685

Open
EffortlessSteven wants to merge 6 commits intoRustCrypto:masterfrom
EffortlessSteven:pkcs1v15-implicit-rejection-retrofit
Open

pkcs1v15: retrofit deterministic implicit rejection for decryption#685
EffortlessSteven wants to merge 6 commits intoRustCrypto:masterfrom
EffortlessSteven:pkcs1v15-implicit-rejection-retrofit

Conversation

@EffortlessSteven
Copy link
Copy Markdown

@EffortlessSteven EffortlessSteven commented Apr 19, 2026

PKCS#1 v1.5 decrypt currently leaks padding validity through Error::Decryption. This PR removes that distinguisher without changing the public decrypt API.

It keeps the existing trait surface and Result<Vec<u8>>, standardizes the outer public-invalid boundary so wrong-length ciphertexts and out-of-range ciphertext representatives still return Err(Error::Decryption), and changes only padding-invalid but otherwise public-valid ciphertexts to return the deterministic rejection symbol.

What changes

  • retrofit the existing PKCS#1 v1.5 decrypt seam in src/algorithms/pad.rs, src/algorithms/pkcs1v15.rs, and src/pkcs1v15.rs
  • add a modulus-width serializer plus local KDK / IRPRF / scan / selector helpers for deterministic implicit rejection
  • keep the public sha2 feature name for compatibility. SHA-256 is now an unconditional internal dependency for PKCS#1 v1.5 implicit rejection, while the sha2 feature continues to gate the SHA-2 reexport and PKCS#1 v1.5 signature OID impls
  • add checked-in Appendix B fixtures and a data-driven vector runner, plus focused regression coverage
  • update PKCS#1 v1.5 docs and changelog to describe the new semantics under the existing API

What does not change

  • no new public safe-decrypt API
  • no fixed-size decapsulation API
  • no public sha2 feature rename or removal
  • no docs.rs feature-policy change
  • no README cleanup
  • no broader PKCS#1 v1.5 policy patch
  • no crate-wide Marvin-closure claim
  • no static/formal verification project
  • no DH = SHA256(D) caching optimization

Evidence

  • Appendix B coverage: B.1 2048-bit, B.2 2049-bit, B.3 3072-bit, B.4 4096-bit
  • Focused regressions: wrong-length and c >= n ciphertexts still return Err(Error::Decryption); identical invalid ciphertexts remain deterministic; distinct invalid ciphertexts produce distinct rejection symbols; valid ciphertexts still decrypt normally; RsaPrivateKey::decrypt(...) and pkcs1v15::DecryptingKey agree on the shared path
  • Rust matrix: cargo test --test pkcs1v15_implicit_rejection --all-features; cargo test --lib --tests --all-features; cargo check --all-features; cargo check --no-default-features
  • Marvin: corrected 100k and 300k baseline/patched runs did not show a strong global separation signal. A focused patched-only 500k follow-up weakened the earlier valid_0 vs zero_byte_in_padding_48_4 suspicion. Current conclusion: supportive evidence, not closure.

Follow-up work

Phase-two follow-ups should stay separate from this retrofit:

  • fixed-size decapsulation API
  • broader PKCS#1 v1.5 deprecation/default-disable work
  • stronger static/formal verification

@EffortlessSteven EffortlessSteven force-pushed the pkcs1v15-implicit-rejection-retrofit branch from 8e622fd to 4a7234a Compare April 19, 2026 06:38
@baloo
Copy link
Copy Markdown
Member

baloo commented Apr 20, 2026 •
edited
Loading

Claude code generated code.
There are at least obvious errors in the Cargo.toml. Rest of the code is not exactly easily reviewable.

There are issues with the lack of use of types (use of Vec instead of an Arrays as used throughout rustcrypto). Honestly this is a time waster, I'd rather we close this.

@EffortlessSteven
Copy link
Copy Markdown
Author

EffortlessSteven commented Apr 21, 2026 •
edited
Loading

On Cargo.toml

The new decrypt path uses Hmac<Sha256> directly for KDK and IRPRF, so hmac is added and sha2 moves to an unconditional internal dependency. The public sha2 feature name stays as an empty shim so existing #[cfg(feature = "sha2")] gates on the SHA-2 reexport and PKCS#1 v1.5 signature OID impls keep compiling.

 

On Vec vs arrays

The public Result<Vec<u8>> is intentional. Earlier guidance on this work (#680) was to retrofit the existing path internally, keep Result where non-padding errors still matter, and avoid adding a new API. Most of the new buffers are modulus-sized or variable-length by construction. The KDK already uses [u8; 32] where the size is actually fixed.

The fixed 256-byte candidate-length block could tighten to [u8; 256]. Pushed 84888dd.

 

Reviewer map

Most of the 16 files are fixtures, tests, or docs. The real production change is four files. Review order:

  1. src/pkcs1v15.rs: semantic boundary. Same public surface. Wrong-length, public-invalid, and non-padding failures still return Err. Padding-invalid but otherwise public-valid ciphertexts now route to deterministic rejection.
  2. src/algorithms/pkcs1v15.rs: core implementation. Focus functions: derive_kdk, irprf_sha256_hmac, select_alt_len_ct, scan_pkcs1v15_encryption_block, select_message_ct. Properties to check: deterministic derivation, k - 11 bound, full separator scan with no early exit. Lengths clamped before use. No secret-selected source slice start. Truncation only after the full read loop.
  3. src/algorithms/pad.rs: one helper, i2osp_modulus_width. Normalizes the decrypted integer to exact modulus width.
  4. src/pkcs1v15/decrypting_key.rs: wrapper and doc alignment.
  5. Tests: tests/pkcs1v15_implicit_rejection.rs, tests/pkcs1v15_implicit_rejection_vectors.rs, tests/support/pkcs1v15_ir.rs. The PEMs and cases.json are Appendix B fixtures.

Happy to narrow further if specific areas are still noisy.

@EffortlessSteven EffortlessSteven force-pushed the pkcs1v15-implicit-rejection-retrofit branch from 68f11f4 to 84888dd Compare April 21, 2026 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@EffortlessSteven @baloo